[clang][SYCL] Add sycl_external attribute and restrict emitting device code #140282

schittir · 2025-05-16T17:04:30Z

This patch is part of the upstreaming effort for supporting SYCL language front end.
It makes the following changes:

Adds sycl_external attribute for functions with external linkage.
Adds checks to avoid emitting device code when sycl_external and sycl_kernel_entry_point attributes are not enabled
Fixes test failures caused by the above changes

This patch is missing diagnostics for the following diagnostics listed in the SYCL 2020 specification's section 5.10.1, which will be addressed in a subsequent PR:
Functions that are declared using SYCL_EXTERNAL have the following additional restrictions beyond those imposed on other device functions:

If the SYCL backend does not support the generic address space then the function cannot use raw pointers as parameter or return types. Explicit pointer classes must be used instead;
The function cannot call group::parallel_for_work_item;
The function cannot be called from a parallel_for_work_group scope.

tahonermann

Thanks for working on this @schittir! I completed an initial pass of all of the code, but still need to look more closely at the documentation updates.

clang/lib/AST/ASTContext.cpp

clang/include/clang/Sema/SemaSYCL.h

clang/test/CodeGenSYCL/kernel-caller-entry-point.cpp

clang/include/clang/Basic/DiagnosticSemaKinds.td

clang/include/clang/Basic/Attr.td

clang/test/SemaSYCL/sycl-external-attribute.cpp

clang/lib/AST/ASTContext.cpp

schittir · 2025-05-19T15:27:54Z

Thanks for working on this @schittir! I completed an initial pass of all of the code, but still need to look more closely at the documentation updates.

Thank you for the initial pass review, Tom!

github-actions · 2025-05-26T22:45:36Z

✅ With the latest revision this PR passed the C/C++ code formatter.

clang/lib/AST/ASTContext.cpp

Support for functions is sufficient for SYCL 2020 spec conformance.

clang/include/clang/Basic/AttrDocs.td

clang/lib/Sema/SemaDeclAttr.cpp

clang/include/clang/Basic/AttrDocs.td

clang/test/CodeGenSYCL/amd-address-space-conversions.cpp

clang/test/CodeGenSYCL/function-attrs.cpp

clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp

clang/test/SemaSYCL/sycl-external-attr-grammar.cpp

clang/test/SemaSYCL/sycl-external-attr.cpp

Co-authored-by: Mariya Podchishchaeva <[email protected]>

tahonermann

Thanks @schittir, here are my initial review comments. I expect to review the newly added tests more closely once these comments are all addressed.

clang/include/clang/Basic/Attr.td

clang/include/clang/Basic/AttrDocs.td

clang/include/clang/Basic/DiagnosticSemaKinds.td

clang/lib/AST/ASTContext.cpp

clang/test/SemaSYCL/sycl-external-attr.cpp

clang/test/SemaSYCL/sycl-external-attr-ignore.cpp

clang/test/SemaSYCL/sycl-external-attr-grammar.cpp

clang/test/SemaSYCL/sycl-external-attr.cpp

clang/test/SemaSYCL/sycl-external-attr-grammar.cpp

clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp

Fznamznon · 2025-07-10T09:08:37Z

Oops new tests are failing.

tahonermann

This is looking good @schittir! I added some comments for minor issues. I still need to review the tests more closely.

clang/include/clang/Basic/Attr.td

clang/include/clang/Basic/DiagnosticSemaKinds.td

clang/lib/Sema/SemaDecl.cpp

clang/test/CodeGenSYCL/kernel-caller-entry-point.cpp

tahonermann

A few more comments. I still haven't finished my review of sycl-external-attr.cpp, but will try to later today or over the weekend.

clang/test/SemaSYCL/sycl-external-attr-grammar.cpp

clang/include/clang/Basic/DiagnosticSemaKinds.td

clang/test/SemaSYCL/sycl-external-attr-ignored.cpp

clang/test/SemaSYCL/sycl-external-attr.cpp

tahonermann

I finished an initial pass through all of the tests. I'll do another round once you've had a chance to catch up on the comments so far.

clang/test/SemaSYCL/sycl-external-attr.cpp

clang/include/clang/Basic/AttrDocs.td

tahonermann

This is looking great @schittir! I just realized that, since we made the change to allow the attribute to be present during host compilation that we should no longer restrict diagnostics to device compilation and that tests should exercise host compilation as well.

I suggested a few FIXME comments, but other than that, I think this is good to go!

clang/lib/Sema/SemaDecl.cpp

clang/test/SemaSYCL/sycl-external-attr-appertainment.cpp

clang/test/SemaSYCL/sycl-external-attr-grammar.cpp

clang/test/SemaSYCL/sycl-external-attr.cpp

schittir · 2025-07-18T15:47:21Z

This is looking great @schittir! I just realized that, since we made the change to allow the attribute to be present during host compilation that we should no longer restrict diagnostics to device compilation and that tests should exercise host compilation as well.

I suggested a few FIXME comments, but other than that, I think this is good to go!

Thank you for the review, Tom!

tahonermann

I love it @schittir, this looks great!

@erichkeane, @bader, please review and lend your approval. Note that some of the changes to the tests are to synchronize them with continued evolution that happened in DPC++ but was not previously upstreamed.

schittir added 2 commits May 16, 2025 10:03

Add sycl_external attribute

abdbf89

Fix test and remove space

f631d7a

tahonermann requested changes May 17, 2025

View reviewed changes

Address review comments #1

128ab1b

Fznamznon reviewed May 27, 2025

View reviewed changes

clang/lib/AST/ASTContext.cpp Outdated Show resolved Hide resolved

schittir added 4 commits May 29, 2025 12:31

Fix conditional and failing tests

118656c

Fix the remaining six failing tests

7c592a4

Fix formatting

90ead01

Merge branch 'main' into sycl_external

195a3cc

Fznamznon mentioned this pull request Jun 6, 2025

lGeneric cast integration for SYCL intel/llvm#18841

Merged

schittir added 3 commits June 9, 2025 13:17

Remove sycl_external attribute support to variables.

a0071d1

Support for functions is sufficient for SYCL 2020 spec conformance.

Rename test file

d20382c

Add tests for sycl_external attribute

65262ba

schittir changed the title ~~Add sycl_external attribute~~ [SYCL] Add sycl_external attribute Jun 10, 2025

Add code examples to sycl_external documentation

770c65e

schittir requested review from tahonermann and Fznamznon June 10, 2025 16:16

schittir marked this pull request as ready for review June 10, 2025 16:16

schittir changed the title ~~[SYCL] Add sycl_external attribute~~ [clang][SYCL Upstreaming] Add sycl_external attribute Jun 10, 2025

Fznamznon added the SYCL https://registry.khronos.org/SYCL label Jun 10, 2025

schittir changed the title ~~[clang][SYCL Upstreaming] Add sycl_external attribute~~ [clang][SYCL Upstreaming] Add sycl_external attribute and restrict emitting code Jun 10, 2025

schittir changed the title ~~[clang][SYCL Upstreaming] Add sycl_external attribute and restrict emitting code~~ [clang][SYCL Upstreaming] Add sycl_external attribute and restrict emitting device code Jun 10, 2025

Fznamznon reviewed Jun 10, 2025

View reviewed changes

schittir and others added 3 commits June 10, 2025 14:48

Merge branch 'main' into sycl_external

328d242

Update clang/lib/Sema/SemaDeclAttr.cpp

aab6f7d

Co-authored-by: Mariya Podchishchaeva <[email protected]>

Address review comments -2

385ea37

Fznamznon changed the title ~~[clang][SYCL Upstreaming] Add sycl_external attribute and restrict emitting device code~~ [clang][SYCL] Add sycl_external attribute and restrict emitting device code Jun 11, 2025

tahonermann requested changes Jun 16, 2025

View reviewed changes

Revert test change

b38e578

Fznamznon reviewed Jul 7, 2025

View reviewed changes

clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp Outdated Show resolved Hide resolved

schittir added 6 commits July 8, 2025 11:02

Merge branch 'main' into sycl_external

1d82fc1

Fix conflict resolution errors.

d751b43

Remove changes introduced from downstream.

2b22ed2

Update diagnostic messages in tests

1b3a198

Undo more downstream changes

568b569

Ungroup diagnostics and add test cases

0ab9ac5

schittir requested review from Fznamznon and tahonermann July 10, 2025 04:24

Merge branch 'main' into sycl_external

19d1660

schittir added 2 commits July 10, 2025 05:51

Fix newly failing tests by adding sycl_external attribute

a70e2df

Add constexpr and consteval test cases

45f7b09

tahonermann requested changes Jul 11, 2025

View reviewed changes

Use existing diagnostic and address other minor comments

4db4101

tahonermann requested changes Jul 11, 2025

View reviewed changes

tahonermann requested changes Jul 12, 2025

View reviewed changes

Add additional test cases and address review comments

931fd76

tahonermann reviewed Jul 15, 2025

View reviewed changes

clang/include/clang/Basic/AttrDocs.td Outdated Show resolved Hide resolved

Add test cases

e34f2a6

schittir requested review from tahonermann, erichkeane and bader July 16, 2025 19:18

tahonermann requested changes Jul 17, 2025

View reviewed changes

Add FIXME comments, enable diagnostics for host, remove a needless decl

13a68d5

Merge branch 'main' into sycl_external

2c6cf7f

schittir requested a review from tahonermann July 18, 2025 15:49

tahonermann approved these changes Jul 18, 2025

View reviewed changes

[clang][SYCL] Add sycl_external attribute and restrict emitting device code #140282

Are you sure you want to change the base?

[clang][SYCL] Add sycl_external attribute and restrict emitting device code #140282

Uh oh!

Conversation

schittir commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tahonermann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

schittir commented May 19, 2025

Uh oh!

github-actions bot commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tahonermann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Fznamznon commented Jul 10, 2025

Uh oh!

tahonermann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tahonermann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tahonermann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tahonermann left a comment

schittir commented May 16, 2025 •

edited

Loading

github-actions bot commented May 26, 2025 •

edited

Loading